-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(core): Externally built Integrations run command configuration from jvm trait #5151
fix(core): Externally built Integrations run command configuration from jvm trait #5151
Conversation
✔️ Unit test coverage report - coverage increased from 35.6% to 35.7% (+0.1%) |
1b7ac66
to
5cd3425
Compare
✔️ Unit test coverage report - coverage increased from 35.6% to 35.7% (+0.1%) |
Could someone trigger the failing test again please 🙏 |
5cd3425
to
053f387
Compare
✔️ Unit test coverage report - coverage increased from 35.6% to 35.7% (+0.1%) |
* change the check to evaluate if the kit is external or not instead of 'camel-k-kit' naming convention * let the possibility for the user to explicitly enable the trait jvm and add a condition to inform the user that it is disabled by default * enable jvm trait explicitly on promote command
053f387
to
e65f156
Compare
✔️ Unit test coverage report - coverage increased from 35.7% to 35.8% (+0.1%) |
I think the PR is good except for known flacky tests. |
@@ -468,6 +469,12 @@ func (o *promoteCmdOptions) editIntegration(it *v1.Integration) *v1.Integration | |||
dst.Spec.Traits.Container = &traitv1.ContainerTrait{} | |||
} | |||
dst.Spec.Traits.Container.Image = contImage | |||
if dst.Spec.Traits.JVM == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this check. We know the promote tests are kind of flaky, no need to add anything in the command IMO. The container has to be initialized because it could raise a nil exception, but not the JVM trait, for which, we must keep the default setting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a flacky test in this case, the promote command run the "promoted" integration with the trait Container.Image
, so it becomes an external kit and does not start correctly. Since the trait was not explicilty set in the original integration, we are relying on default behavior, which will be different for original and promoted integration. This aims to fix this issue.
The alternative would be to add support for the traits on the comand options or add the copy of the integration kit created for the image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. We can proceed then.
@@ -468,6 +469,12 @@ func (o *promoteCmdOptions) editIntegration(it *v1.Integration) *v1.Integration | |||
dst.Spec.Traits.Container = &traitv1.ContainerTrait{} | |||
} | |||
dst.Spec.Traits.Container.Image = contImage | |||
if dst.Spec.Traits.JVM == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. We can proceed then.
Closes #5112
Release Note